UL&S: Add TextLinkTableViewCell to SiteAddressViewController #308
Conversation
Don't allow direct access to IBOutlet properties
It adds the first responder too soon in the view lifecycle
buttons in the configuration file
iPhone SE (2nd gen) simulator lags when this method is added and the table has `didSelectRow` delegate method available.
So that it doesn't disrupt WPiOS and WCiOS
|
@ScoutHarris ready for another review. For dynamic text on the button in the TextLinkButtonTableViewCell, I first tried using a text label in a row, then implementing the Anyway, TIL that buttons can have dynamic text too. It's not a control that lives in InterfaceBuilder. It has to be set programmatically. It's working now. I also touched up the controls for VoiceOver, though I don't plan on asking reviewers to test that until the whole screen is ready to go.
Found a bug in NUXButton. When the dynamic text shrinks back down, the NUXButtons do not respond to the change. |
| @@ -123,31 +123,57 @@ private extension SiteAddressViewController { | |||
| /// | |||
| func loadRows() { | |||
| rows = [.instructions, .siteAddress] | |||
| let displayHintButtons = WordPressAuthenticator.shared.configuration.displayHintButtons | |||
| if displayHintButtons { | |||
There was a problem hiding this comment.
Couldn't this just be if WordPressAuthenticator.shared.configuration.displayHintButtons {?
| /// | ||
| extension Collection { | ||
| /// Returns the element at the specified index if it is within bounds, otherwise nil. | ||
| subscript (safe index: Index) -> Element? { |
There was a problem hiding this comment.
Maybe I missed something, but is this used by anything in this PR?
There was a problem hiding this comment.
Artifact from trying out a label and setting didSelectRowAt. Will delete!
|
Was confused about why |
|
I only see one issue. With large text on an iPhone (ex: 11 Pro Max), the hint text is truncated. I would expect it to wrap.
I figure you might have the same issue with the error label, so I'd be ok with resolving this in a follow up PR. It's up to you, but I think we can consider this GTG assuming that will be fixed later. |
|
Find yo' address 😂 . Thanks for the review! I will make the button text wrap in the next PR. |
Ref. #283
This PR refines the code style to be consistent among the new table cells and creates another base UI piece, TextLinkTableViewCell, which is a plain button with text. If only one property is needed on a cell, the variable is made public and the value can be assigned. For more than one property on a cell, a
configurationmethod is created so the different parameters can be passed in.Concerns: the naming isn't consistent. The thing is a hint button according to the user, but a plain button in interface builder.
TextLinkButtonTableViewCell?HintButtonTableViewCell?To test
bundle exec pod installTextLinkTableViewCellto SiteAddressViewController WordPress-iOS#14372